Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wiring context for EVM Keys API #11800

Merged
merged 20 commits into from
Feb 27, 2024
Merged

Wiring context for EVM Keys API #11800

merged 20 commits into from
Feb 27, 2024

Conversation

patrickhuie19
Copy link
Contributor

@patrickhuie19 patrickhuie19 commented Jan 17, 2024

Wiring context through the EVM Keys API. Makes the db sql middleware added in #12097 more useful.

Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

core/scripts/go.mod Outdated Show resolved Hide resolved
@patrickhuie19 patrickhuie19 force-pushed the feature/sql-tracing branch 2 times, most recently from e6bbbe8 to 8448f0c Compare February 16, 2024 21:51
@patrickhuie19 patrickhuie19 changed the title adding sqlx tracing to core Wiring context for EVM Keys API Feb 20, 2024
common/txmgr/resender.go Outdated Show resolved Hide resolved
common/txmgr/test_helpers.go Outdated Show resolved Hide resolved
core/cmd/shell_local_test.go Outdated Show resolved Hide resolved
core/web/eth_keys_controller.go Outdated Show resolved Hide resolved
@@ -177,7 +177,7 @@ func newFunctionsContractTransmitter(contractVersion uint32, rargs commontypes.R
if sendingKeysLength > 1 && s == effectiveTransmitterAddress.String() {
return nil, errors.New("the transmitter is a local sending key with transaction forwarding enabled")
}
if err := ethKeystore.CheckEnabled(common.HexToAddress(s), configWatcher.chain.Config().EVM().ChainID()); err != nil {
if err := ethKeystore.CheckEnabled(ctx, common.HexToAddress(s), configWatcher.chain.Config().EVM().ChainID()); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should create a ticket to follow up with investigating each constructor that now requires context. Hopefully they are all just database related, but even that can be a problem, since a temporary hiccup during start-up could short-circuit things arbitrarily.

common/txmgr/txmgr.go Outdated Show resolved Hide resolved
common/txmgr/txmgr.go Outdated Show resolved Hide resolved
core/services/job/orm.go Outdated Show resolved Hide resolved
core/cmd/shell_local.go Outdated Show resolved Hide resolved
…ontext() in web routes, and marks TODOs where needed
@patrickhuie19 patrickhuie19 added this pull request to the merge queue Feb 26, 2024
Merged via the queue into develop with commit 64df78e Feb 27, 2024
97 checks passed
@patrickhuie19 patrickhuie19 deleted the feature/sql-tracing branch February 27, 2024 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants